fix: always include temperature parameter for OpenAI compatible providers #7582
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ders - Modified base-openai-compatible-provider.ts to always include temperature with fallback to defaultTemperature - Modified openai.ts to always include temperature with fallback to 0 (or DEEP_SEEK_DEFAULT_TEMPERATURE for DeepSeek reasoner models) - Updated tests to expect temperature to always be included with appropriate defaults - This fixes TabbyApi/ExLlamaV2 crashes when temperature is undefined/null Fixes #7581
dosubot
bot
added
size:M
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| params.temperature = this.options.modelTemperature | ||
| // Always include temperature to prevent TabbyApi/ExLlamaV2 crashes | ||
| // Use explicitly set temperature, or fall back to defaultTemperature (which defaults to 0) | ||
| temperature: this.options.modelTemperature ?? this.defaultTemperature, |
Contributor
Author
There was a problem hiding this comment.
Good implementation of the temperature fallback chain! The comment clearly explains why this is needed. However, I noticed the completePrompt method in this same file (line 117-134) still doesn't include the temperature parameter - this could still cause TabbyApi/ExLlamaV2 crashes when using the completion endpoint.
| requestOptions.temperature = DEEP_SEEK_DEFAULT_TEMPERATURE | ||
| // Always include temperature to prevent TabbyApi/ExLlamaV2 crashes | ||
| // Use explicitly set temperature, or DeepSeek default for reasoner models, or fall back to 0 | ||
| temperature: this.options.modelTemperature ?? (deepseekReasoner ? DEEP_SEEK_DEFAULT_TEMPERATURE : 0), |
Contributor
Author
There was a problem hiding this comment.
Good fix for the streaming case! This ensures temperature is always included with appropriate defaults.
| : [systemMessage, ...convertToOpenAiMessages(messages)], | ||
| // Always include temperature to prevent TabbyApi/ExLlamaV2 crashes | ||
| // Use explicitly set temperature, or DeepSeek default for reasoner models, or fall back to 0 | ||
| temperature: this.options.modelTemperature ?? (deepseekReasoner ? DEEP_SEEK_DEFAULT_TEMPERATURE : 0), |
Contributor
Author
There was a problem hiding this comment.
Excellent - non-streaming mode also properly includes temperature now. The fallback chain (user setting -> DeepSeek default -> 0) is well thought out.
| model: model.id, | ||
| messages: [{ role: "user", content: prompt }], | ||
| // Always include temperature to prevent TabbyApi/ExLlamaV2 crashes | ||
| temperature: this.options.modelTemperature ?? 0, |
Contributor
Author
There was a problem hiding this comment.
Good catch on the completePrompt method! This ensures consistency across all API calls.
hannesrudolph
added
the
Issue/PR - Triage
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
Member
|
Closing in favor of #7594 |
github-project-automation
bot
moved this from PR [Needs Prelim Review]
to Done
in Roo Code Roadmap
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes the TabbyApi/ExLlamaV2 crash issue introduced after v3.25.20 where the temperature parameter was being passed as
None/undefinedinstead of a valid float value.Problem
After commit 090737c (#7188), the temperature parameter was omitted when not explicitly set to allow backend services to use their configured defaults. However, some backends like TabbyApi/ExLlamaV2 expect a temperature value and crash when receiving
None.Solution
base-openai-compatible-provider.tsto always include temperature with fallback todefaultTemperatureopenai.tsto always include temperature with fallback to 0 (orDEEP_SEEK_DEFAULT_TEMPERATUREfor DeepSeek reasoner models)Testing
Impact
Fixes #7581
Important
Ensure temperature parameter is always included with defaults in OpenAI-compatible providers to prevent crashes.
base-openai-compatible-provider.tsandopenai.tsto prevent crashes in TabbyApi/ExLlamaV2.defaultTemperatureor provider-specific defaults (OpenAI: 0, Groq: 0.5, Roo: 0.7, DeepSeek Reasoner: 0.6) if not explicitly set.groq.spec.ts,openai.spec.ts, androo.spec.tsto verify temperature inclusion with appropriate defaults.This description was created by
for 8d2f767. You can customize this summary. It will automatically update as commits are pushed.